test(jest): opt-in to shorter snapshot format #4843
Conversation
jestjs/jest#11654 ...waiting for the release of jest
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3f5ec1d:
|
| @@ -417,7 +417,7 @@ See https://www.algolia.com/doc/api-reference/widgets/configure/js/`); | |||
|
|
|||
| search.start(); | |||
|
|
|||
| await runAllMicroTasks(); | |||
| await Promise.resolve(); | |||
There was a problem hiding this comment.
What if we use wait(0) here too?
Or does it make sense to bring runAllMicroTasks back and replace its implementation with wait()?
There was a problem hiding this comment.
we definitely can use wait(0), but I'm not sure if it really means the same, this code happens before a timeout runs
There was a problem hiding this comment.
While I understand what you mean, I fear this subtleness that might not make any real life difference could complicate writing tests.
There was a problem hiding this comment.
makes sense, I'll check whether those tests can be done with wait too
| @@ -1047,7 +1047,8 @@ describe('dispose', () => { | |||
|
|
|||
| search.start(); | |||
|
|
|||
| await runAllMicroTasks(); | |||
| await Promise.resolve(); | |||
| await Promise.resolve(); | |||
There was a problem hiding this comment.
Calling it once didn't pass the test?
There was a problem hiding this comment.
yes, there's two micro-tasks that need to happen, this implementation makes that clearer than wait(0) IMO
There was a problem hiding this comment.
oh I see, but I feel like this is testing too much detail here because what we want to know is if onRender is called after rendering happens. We do not want to test if onRender is called two micro-tasks after start().
So I'm wondering what if we
const runAllMicroTasks = () => wait(0)and use it normally- use
await Promise.resolve()when we need to test specific timing issue?
| // Wait for the `render` | ||
| await Promise.resolve(); | ||
| await Promise.resolve(); | ||
| await wait(search._stalledSearchDelay); |
Summary
Changing from snapshot to pure expect is useful sometimes if you know the assertion will be pretty hard to write, but before this PR requires a lot of editing. Since we use this shorter format for snapshots, no more "Array" or "Object" will be printed in snapshots.
Result
ObjectorArrayin snapshots 🙌This PR is easier to review as their individual commits